Skip to content

test: cross-examine the six transaction claims #1021 rests on - #1039

Draft
grypez wants to merge 48 commits into
sirtimid/vat-lifecycle-consistency-v2from
grypez/crank-claims-repro
Draft

grypez wants to merge 48 commits into
sirtimid/vat-lifecycle-consistency-v2from
grypez/crank-claims-repro

Conversation

@grypez

@grypez grypez commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Explanation

Cross-examination of the six load-bearing claims in #1021, as executable tests.
Each is a repro, not a fix: five files, ten tests, one commit per file. Seven
fail and three pass, and the split is the point — the passing three guard a fix
this branch already made.

Read the base ref carefully. #1021 is the PR whose claims these examine, but
#1022 and #1023 share no commits with it (811ee0b23, #1021's core fix, is not
an ancestor of #1022; the merge-base is 180e6ac47 on main, before #1021
starts). They are parallel re-applications of overlapping work, not a stack, so
"against the tip" and "against #1021" are different questions with different
answers. This PR targets the tip, sirtimid/vat-lifecycle-consistency-v2, and
every outcome below was measured there.

# Claim under test File Result at this base
1 Releasing crank in endCrank is the crank's one commit point nodejs.savepoint-interleaving.test.ts 3 fail
2 Flush last, then audit — so no caller is answered before the fallible work KernelQueue.audit-ordering.test.ts 1 fail
3 A failed ROLLBACK TO discards the whole transaction, in both drivers nodejs.transaction-survival.test.ts 2 fail
4 Every kref in maybeFreeKrefs was put there by this crank crank.cross-crank-gc.test.ts 3 pass
5 Reverting caches is safe on the failed-rollback path too (covered by 3 and 4) see comment
6 commitIfNeeded leaves no transaction behind wasm.transaction-survival.test.ts 1 fail

Claim 4 is false against #1021 and true here: eaa71ac00 on this branch
snapshots maybeFreeKrefs per savepoint and restores it rather than clearing.
Those three tests pass and are offered as a regression guard on that fix — they
fail against #1021's head, where the same code calls ctx.maybeFreeKrefs.clear().

Claim 6 runs the other way. This branch's wasm commitIfNeeded clears _inTx
before the COMMIT but never aborts, so a failed COMMIT leaves an ownerless
transaction — the gap b90e7a5e5 closes on #1021 and which this branch, not
descending from it, does not have. Whichever lands second has to carry that fix
across.

Claims 1, 3, and the nodejs half of 6 are unchanged everywhere and fail against
main, #1021, and this tip alike.

A per-claim walkthrough — what the claim is, why it matters, how to evaluate it,
what came out — is in the review comments, one per claim.

No production code and no changelog entries: this PR only adds tests.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary — n/a, tests only
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

sirtimid and others added 30 commits August 13, 2026 19:37
Creating an import c-list entry changed no refcount while tearing one
down decremented both, and `initKernelObject` compensated by minting
every object at (1, 1). That constant is correct for exactly one
importer, which is why nothing caught it: with two importers a live
capability gets dropped and retired out from under a holder, and the
same unit is claimed by both an importer's drop and the owner's
termination, so cleanup underflows and leaves a vat half-cleaned.

Restore the increment and rebase the baseline to (0, 0), matching
SwingSet, so `collectGarbage` — already a faithful port — receives the
inputs it was written for.

Build the invariant checker first, since every existing compensation
becomes a double-count the moment the increment lands. It recomputes
each kref's counts from ground truth (c-list entries and their reachable
flags, run-queue and promise-queue messages, promise resolution values,
pins) and reports drift in both directions: too low collects a live
capability, too high leaks it. Enabled via `Kernel.make`'s
`auditRefCounts` and run after every crank; on in kernel-test.

The audit found four more unbalanced paths that the phantom baseline had
been absorbing, each fixed here: a delivered message charged its target
against the routed kref rather than the run-queue item's own, so a
message routed through a resolved promise decremented an object nobody
charged and leaked the promise; a notification leaked its reference on
both early-return paths and decremented promises retired alongside it
that nobody had taken; a message queued on an unresolved promise
duplicated every reference it carried on re-enqueue; and `resolve|kpid`
incremented with no matching release.

Two things the baseline was silently standing in for, now explicit: vat
roots are pinned for the lifetime of their vat (a root is addressable
whether or not anyone imports it), and GC action delivery moves the
kernel's own c-list so a dropped export's flag clears and retired
entries don't outlive their objects.

Also fixes the stale `cle.`/`clk.` key prefixes in
`getPromisesByDecider` and `deleteEndpoint`, which stopped matching the
`${endpointId}.c.` layout. `getPromisesByDecider` matched nothing, so
promises a terminating vat was deciding were never rejected — load
bearing here, because releasing a promise's unsettled reference is what
makes the cleanup path's accounting add up.

Refcounts are persisted, so counts written under the old scheme are
recomputed from ground truth on first open, keyed off a new
`refCountScheme` entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Prettier wanted a blank line before the entry following a nested bullet,
and the entries still cited #1010, which this PR replaces.
…ring

`retireKernelObjects` deletes an object and queues a `retireImport` for each
importer in the same breath, so until that action is delivered an importer's
c-list entry names a kref the kernel has already dropped. The audit counted
those entries as holders and reported a violation against the collector's own
output — and since `assertRefCountsIfAuditing` throws from inside the crank,
that killed the run loop for good.

Reachable from an ordinary `terminateVat` while a surviving vat holds the
dying vat's export in liveslots' dropped-but-recognizable state. No current
test produced it; found by Cursor Bugbot on #1020 and reproduced against the
real store.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebasing the baseline to (0, 0) made every reference explicit, which
exposed the holders that were never references at all. An ocap URL
carries its kref inside an encrypted bearer token and nothing else, so
the kernel cannot discover from its own state that a holder exists:
`issueOcapURL` took no reference of any kind. Under the old baseline
nothing exported was collectable and it never showed; at (0, 0) the
target is collected as soon as the message that carried it to the issuer
is delivered, and the URL names a dead capability. The audit is silent
on it by construction — the object genuinely has no holder it can see.

Retain the target when the URL is issued, before the token exists, since
the token is unretractable once it does. One pin per kref however many
URLs name it, and no release: the token is persistent and unexpiring, so
`revoke` is how the capability dies. Pinning also puts the holder inside
the reference graph, so the audit can see it rather than being taught to
excuse it.

The same shape had a second door. `incrementRefCount` has no
`kernelRefExists` guard where `decrementRefCount` does, so importing a
deleted kref read its missing counts as (0, 0) and wrote them back,
resurrecting a live-looking object with no owner — deliverable to by
nobody, and endorsed by the audit, since the new c-list entry is a
legitimate holder for exactly the count it finds. Reached by redeeming a
URL issued for an object since collected. Guard the point of corruption,
`translateRefKtoE`, rather than `incrementRefCount` itself: creating an
entry for a deleted kref is the invariant, and releasing a reference to
something already gone is how GC teardown is allowed to race deletion.

Also release a vat's root pin when `deleteSubcluster` retires vats that
never ran here. It bypasses `stopVat`, so nothing released the pin
`launchVat` took in the incarnation that did run them, leaving the root's
count permanently above zero and `pinnedObjects` naming a vat that no
longer exists. `stopVat` and `deleteSubcluster` now share
`releaseVatRootPin`.

Vat root pinning had no unit coverage at all, so pin-on-launch,
release-on-terminate and keep-across-restart are asserted now; the last
is what the comment claims and what would break silently. Restores the
`maybeFreeKrefs` assertion on `forgetEndpointImports`' ownership-migrated
branch, which lost its `not.toHaveBeenCalled` when that branch stopped
returning early.

Corrects three claims that the (0, 0) birth falsified and that shipped as
documentation: both `KernelServiceManager` comments asserting its delete
branch cannot fire, when it now does, and a changelog entry asserting
(1, 1) birth two dozen lines above one asserting (0, 0). `recomputeRefCounts`
no longer describes itself as a migration; nothing calls it, and opening
an existing store does not migrate one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Retaining before minting is right: minting awaits, so a collection crank can run in that window. But nothing undid the retention when minting then failed. A rejected kernel-service call is reported to the caller rather than thrown out of the crank, so the crank commits and the pin outlives the kernel that took it, naming a URL that never existed.

retainForOcapURL now reports whether this call took the pin, and undoOcapURLRetention unwinds one that never backed a URL. Guarded on the ledger rather than the pin list, so it can only remove the pin it put there: a kref some live URL already names keeps the pin that URL depends on, and a vat root keeps its lifetime pin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The retention was deduplicated by kref, and the failure path undid it if this
call was the one that took it. Minting awaits, though, so issuances for the
same target overlap: a second `issue` can mint a URL while the first is still
in flight, having taken no retention of its own because the ledger already
named the kref. If the first then fails it unwinds the retention the second's
live URL depends on, and collection can take the capability out from under it.

The ledger is a multiset now, one entry and one pin per issuance, so a failed
mint releases only what it took. Pins were already a multiset, and each pin
here is either released by its own failure or held by its own live URL, so
none is left unreleasable — the concern that motivated deduplicating.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eight tests, all currently failing, for three defects that landed with
#1005. They change no production code: each one states the invariant the
fix has to restore, so the diff that repairs them is the specification
being met rather than a claim about it.

`releaseSavepoint` was never hardened the way `rollbackSavepoint` was in
that PR. A RELEASE that throws leaves the savepoint on the stack and the
transaction open with nothing that will ever commit or abort it, so every
later write on the connection joins it, reports success, and vanishes on
close — verbatim the failure mode #1005 documents for the other door. The
driver tests sit beside their rollback counterparts so the asymmetry is
visible in place. `endCrank` gets the companion case: it now settles its
waiters in a `finally`, which is right, but it also leaves the savepoint
listed, so the next crank numbers its savepoint `t1` against a database
that still has `t0`.

`#processCrankResult` does fallible work after the crank's transactional
boundary has already been crossed. On the success path `#flushCrankBuffer`
settles the promise `enqueueMessage` handed an external caller, and only
then can `#terminateVat` throw and have the new catch roll the crank back
— so the caller keeps an answer computed from state the store discarded,
and a restart delivers the message again. On the abort path the rollback
ends the transaction, so `#terminateVat` and `collectGarbage` autocommit
piecemeal and the second rollback the flag correctly suppresses would
have had nothing left to undo either way. The invariant is stated as "the
rollback is the last thing the crank asks of the store", which leaves the
choice of remedy open.

The wasm driver tracks `_inTx` itself rather than reading it from SQLite,
so a failed abort inside the new catch is the one case that can leave it
disagreeing with the database. Left true, `beginIfNeeded` is a no-op from
then on and the next `createSavepoint` runs in autocommit mode, where the
matching RELEASE commits (Agoric/agoric-sdk#8423, already cited two lines
above the code) and no rollback can undo the delivery. The second test
runs that next `createSavepoint` and asserts the BEGIN, so the corruption
path is observable instead of argued.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three transaction-integrity defects, all in the same family: a store call
fails, and the layer above goes on as though its bookkeeping still matched
the database.

- `releaseSavepoint` (both SQLite drivers) discards the enclosing
  transaction when `RELEASE` fails, as `rollbackSavepoint` already does
  when `ROLLBACK TO` fails. Left as it was, the savepoint stayed on the
  stack and the transaction open with nothing to ever commit or abort it,
  so every later write on the connection joined it, reported success, and
  vanished on `close()`.
- `releaseAllSavepoints` forgets its savepoints even if the release
  throws, as `rollbackCrank` already does. A savepoint left listed had the
  next crank number its savepoint `t1` while the database still had `t0`,
  from which point every release and rollback aimed one crank past the one
  it meant to end.
- The wasm driver stops believing it is in a transaction when an abort
  fails. `_inTx` is tracked in the driver rather than read from SQLite, and
  an abort usually fails because SQLite already rolled back on its own.
  Left true, `beginIfNeeded` was a no-op from then on and the next
  `createSavepoint` ran in autocommit mode, where its `RELEASE` commits
  (Agoric/agoric-sdk#8423) and no later rollback could undo the delivery.

And the crank boundary itself, in two parts:

- A crank now takes two savepoints. Rolling back to the outermost one
  discards the enclosing transaction, so the work an aborted crank still
  owes — terminating the vat whose delivery failed, collecting garbage —
  was autocommitting statement by statement, beyond the reach of any later
  rollback. That work has to follow the rollback, since the worker is gone
  and the store must not go on believing the vat is alive, so it is the
  rollback that spares the transaction. Releasing the outer savepoint in
  `endCrank` is now a crank's one commit point.
- `#flushCrankBuffer` runs last, after everything that can still fail.
  It settles the promise `enqueueMessage` handed an external caller,
  reading the result out of the store; rolling the crank back after that
  left the caller holding an answer computed from state the store had
  discarded, and a restart would deliver the message again.

Tests for the first three defects are Ryan's, from #1011. The two crank
tests there specify the remedy as "the rollback is the last thing the
crank asks of the store", which reordering the fallible work before it
would satisfy — but that rollback would then undo the vat termination.
They are restated here as the invariant the fix does hold.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
`should trigger GC syscalls through bringOutYourDead` scheduled one reap
and then ran three cranks. `scheduleReap` dedupes, so that bought one
`bringOutYourDead`, not three — and an import is only reported as dropped
once the engine has collected the vat's presence and run its finalizer,
which the forced GC pass inside `bringOutYourDead` cannot guarantee on the
first attempt. When it hadn't, no further reap was ever scheduled and the
refcount stayed where it was: `expected 2 to be 1`, as on main in
31081630878.

Each attempt now schedules its own reap and stops as soon as the kernel's
bookkeeping catches up, so the common case is one crank rather than three.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A failed `ROLLBACK TO` discards the whole transaction, taking every
savepoint with it — not just the one rolled back to. `rollbackCrank`
truncated `ctx.savepoints` to the rolled-back ordinal regardless, which
was correct while a crank took one savepoint at ordinal 0 and cleared the
list, but leaves `['crank']` listed now that the delivery sits at ordinal
1.

`endCrank` then releases a `t0` the database no longer has, and throws
"No such savepoint: t0" from the run loop's `finally` — replacing the
failure that actually killed the kernel, with no `cause`. That is the
masking this branch's own error-preservation exists to prevent.

Clear the list on the throwing path, truncate to the ordinal only on
success.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion

Both drivers recover from a failed savepoint operation by discarding the
enclosing transaction, and swallow any error from that abort so the
savepoint failure stays the one reported. That part is right, but it left
the abandoned transaction entirely silent: on the nodejs driver, where
`inTransaction` is read from SQLite, the next crank's `beginIfNeeded`
sees the transaction still open, skips its `BEGIN`, and commits the dead
crank's writes alongside the new crank's.

Nothing here can repair that, so at least record it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Moving `#invokeKernelSubscription` out of the enqueue loop and after it
was the one production change on this branch with no test: reverting
`#flushCrankBuffer` to its interleaved form left all 2412 ocap-kernel
tests passing.

Same hazard as the crank-level ordering a few tests up, one level down —
`#enqueueRun` is store work and can fail part-way, so answering the first
caller while the second enqueue is still ahead hands out a result the
crank's rollback then discards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five comments on this branch asserted more than the code holds:

- `wasm.ts` claimed a stale `_inTx` meant "no later rollback can undo the
  delivery". False: a savepoint created in autocommit mode does open a
  transaction, and an inner savepoint still rolls back. The real cost is
  that writes outside a savepoint autocommit one statement at a time, and
  the outermost `RELEASE` commits. The "an abort typically fails because
  SQLite already rolled back" premise was unsupported and isn't the
  reason for the reorder — the reason is simply that the abort can throw.
- `#processCrankResult` said "the worker is already gone" ahead of the
  call that kills the worker.
- The flush was described as running "once nothing fallible remains".
  It doesn't: `#terminateVat` resolves the dying vat's promises through
  `resolvePromises`, which defaults to `immediate` and invokes their
  kernel subscriptions before `collectGarbage`. Reachable without an
  abort, via a clean `exitVat`. Recorded rather than fixed — closing it
  changes termination semantics, not crank ordering.
- "Only `delivery` is ever rolled back" is true of the run loop but not
  of the tests. Scoped, and the ordinal coupling it depends on is now
  stated: `endCrank` releases `t0` by position, so `crank` must stay
  first.
- `reapImporterUntil` credited `scheduleReap` deduping for the old
  one-BOYD behaviour; it was `nextReapAction` shifting the single entry
  off, leaving the later cranks nothing to do.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment the non-obvious why, in the shortest form that carries it. The
two-savepoint rationale was re-argued in full in four places; the tests
now point at `#runLoop` and `#processCrankResult` instead of restating
them, and the hazard block duplicated across both driver test files is a
line. No reasoning removed, only the retelling.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lback

A database rollback cannot reach two pieces of state, so `rollbackCrank`
now reverts both itself.

Every `provideCachedStoredValue` answers reads from a closure and only
writes through to kv. Reverting the database therefore left the closure
holding the abandoned crank's value, and the next `set` persisted it.
`processGCActionSet` takes an action out of the set before delivering it,
so an aborted delivery lost the action outright rather than retrying it.
`reapQueue` was exposed the same way.

`maybeFreeKrefs` lives in RAM, so nothing reverted it either. Its entries
are collection candidates only because of the decrements the rollback
undid, and a later `collectGarbage` threw outright on a promise the
rollback had deleted, killing the run loop.

No live bug either way: every `abort` `#deliverGCAction` returns is paired
with a `terminate`, which is what made losing the action harmless. The
comment there claimed the rollback restored the action, which is the thing
a future reader would trust when adding an abort path that isn't paired
with a termination; it now states the real causality.

The cached values are declared once so that initialization and the
refresher cannot disagree about which ones exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Failing repro, not a fix.

## The issue

`rollbackIfNeeded` was corrected in #1012 to clear `_inTx` *before* stepping
the abort, because the abort can throw and `_inTx` is tracked in the driver
rather than read from SQLite. `commitIfNeeded` has the identical shape and was
left alone:

    function commitIfNeeded(): void {
      if (db._inTx && db._spStack.length === 0) {
        sqlCommitTransaction.step();   // can throw
        sqlCommitTransaction.reset();
        db._inTx = false;              // ...so this never runs
      }
    }

A COMMIT that throws leaves `_inTx` true against a database that may hold no
transaction. `beginIfNeeded` is then a no-op forever after, so the next
`createSavepoint` issues its SAVEPOINT outside a transaction — and a savepoint
taken outside a transaction commits when it is released
(Agoric/agoric-sdk#8423). That is the hazard the whole `beginIfNeeded` dance
exists to prevent, and `commitIfNeeded` is reached from `releaseSavepoint`,
which is the crank's commit point. The writes that leak are a whole crank's.

The nodejs driver is unaffected, for the same reason it was unaffected by the
abort case: it reads `db.inTransaction` live from SQLite.

Worth noting that the comment introduced above `stops believing it is in a
transaction when the abort fails too` asserts that a failed abort is "the one
case that can leave `_inTx` disagreeing with the database". This is the second
case, so that comment needs correcting along with the code.

## What we hope to see instead

`releaseSavepoint` still throws the COMMIT failure, but `_inTx` is false
afterwards, so the next `createSavepoint` opens a transaction of its own
instead of creating a bare savepoint. Same two-line reorder as
`rollbackIfNeeded`, and the "one case" comment updated.

## Current failure

    AssertionError: expected true to be false
      packages/kernel-store/src/sqlite/wasm.test.ts
      > stops believing it is in a transaction when the commit fails

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Failing repro, not a fix.

## The issue

#1012 fixes one error-masking path at the start of a dying crank and opens
another at its end.

Before the two-savepoint scheme, `rollbackCrank('start')` emptied
`ctx.savepoints`, so `endCrank` -> `releaseAllSavepoints` was a guaranteed
no-op on the dying path: nothing to release, nothing that could throw. Now
`rollbackCrank('delivery')` truncates to the ordinal and leaves `['crank']`
behind (crank.ts:56, deliberately — that is what keeps the transaction open for
the work an aborted crank still owes). So `endCrank` issues a real
`RELEASE t0`, which commits, which can fail.

`#runLoop` calls it from a bare `finally`:

    } finally {
      this.#kernelStore.endCrank();
      ...
    }

A throw there replaces the pending exception. The disk error that actually
killed the kernel is discarded — not demoted to `cause`, discarded — and
`run()` rejects with the release failure instead. `#failRunLoop` records that,
so `getRunLoopStatus().detail` loses the root cause too, and
`onRunLoopFailure` — what the daemon logs as fatal — gets the wrong error.

A/B against origin/main with the same repro: main reports `crank exploded`,
this branch reports `database is gone` with `cause: undefined`.

This is the same class of bug as the `No such savepoint: t0` masking that
82b88ce fixes, and the same class the `reports both failures when the
rollback also fails` test above already guards on the other path.

## What we hope to see instead

Whatever names the release failure, the error that killed the crank stays
reachable. The rollback path already has the shape to copy:

    throw new Error(
      `Run loop died and its crank could not be rolled back: ${...}`,
      { cause: error },
    );

The assertion is deliberately fix-agnostic — it walks the `cause` chain — so
either wrapping `endCrank`'s failure with the original as `cause`, or reporting
it and rethrowing the original, will satisfy it.

## Current failure

    AssertionError: expected [ Error: database is gone ]
      to include Error: crank exploded
      packages/ocap-kernel/src/KernelQueue.test.ts
      > reports both failures when endCrank also fails

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Failing repro, not a fix.

## The issue

#1012 replaces four silently-swallowed aborts with `logger?.error(...)` in the
SQLite drivers, and its description says: "Four swallowed aborts were silent.
Now logged." They are not. No production call site passes a `logger` to
`makeSQLKernelDatabase`, so every one of those calls is dead code:

    packages/kernel-node-runtime/src/kernel/make-kernel.ts:63
    packages/kernel-browser-runtime/src/kernel-worker/kernel-worker.ts:47
    packages/kernel-test-local/src/lms-chat.ts:30
    packages/kernel-node-runtime/test/helpers/remote-comms.ts:172

`make-kernel.ts` is the clearest case: it builds a `rootLogger` and hands
sub-loggers to `NodejsPlatformServices` and to `Kernel.make`, then constructs
the store with `{ dbFilename }` alone. The store is the one collaborator that
gets no logger. Nor does any test pass one, which is why the gap survived
review.

This matters more than a missing log line. On the nodejs driver a failed abort
leaves `db.inTransaction` true with nothing that will ever commit or abort it,
so later writes on that connection join a transaction that vanishes on close.
The driver's own comment concedes "Nothing here can repair that" — the log is
the entire remedy, and it does not reach anyone.

`logger?.error` is the right convention for this package; the injection is what
is missing.

## What we hope to see instead

`makeKernel` passes a tagged sub-logger to `makeSQLKernelDatabase`, as it
already does for its other collaborators — something like
`rootLogger.subLogger({ tags: ['store'] })`. The other three call sites want the
same treatment, and are worth covering once this one is fixed.

## Current failure

    AssertionError: expected "vi.fn()" to be called with arguments:
      [ ObjectContaining{…} ]
    -     "logger": Any<Logger>,
      packages/kernel-node-runtime/src/kernel/make-kernel.test.ts
      > gives the kernel store a logger

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Failing repro, not a fix.

## The issue

#1012 hardens `releaseSavepoint` so that a failed `RELEASE` discards the
enclosing transaction, clearing the driver's `_spStack` on the way. Two callers
it does not touch depend on the old behaviour, and both are now worse off than
before the change.

`RemoteHandle.handleRemoteMessage` releases inside the `try` and rolls back in
the `catch`:

    this.#kernelStore.setRemoteHighestReceivedSeq(this.remoteId, seq);
    this.#kernelStore.releaseSavepoint(savepointName);   // fails
  } catch (error) {
    this.#kernelStore.rollbackSavepoint(savepointName);  // "No such savepoint"
    throw error;                                        // never reached
  }

Since the release already cleared the stack, the rollback throws
`No such savepoint: receive_r0_1`, which escapes the `catch` and replaces the
real failure. Not demoted to `cause` — replaced. `RemoteManager` has the same
shape at its `peerIncarnation_*` savepoint.

A/B verified against origin/main with a real driver: main's rollback succeeds
and `database or disk is full` propagates; on this branch the caller gets the
missing-savepoint error instead. So the PR description's "the release failure
still propagates" holds for the crank path it fixed and not for these two.

`crank.ts:57-63` shows the author recognised exactly this hazard — a stale
savepoint list producing `No such savepoint` over the real error — and fixed it
for the crank only. The remote paths were missed because nothing exercised them.

Note the secondary effect these tests don't reach: `ctx.savepoints` still lists
the crank's own savepoints after this, so the next `endCrank` throws
`No such savepoint: t0` over whatever is left of the failure.

## What we hope to see instead

The failure the database reported is what reaches the caller. Any of these does
it, and the assertion doesn't care which:

- move the release out of the `try`, so a release failure isn't followed by a
  rollback attempt at all
- have the `catch` tolerate a rollback that reports a savepoint already
  discarded, rethrowing the original either way
- make the driver's discard leave the name rollback-able as a no-op

The mock models the drivers' bookkeeping rather than the expected outcome, so it
is `RemoteHandle`'s error handling under test, not the mock's.

## Current failure

    AssertionError: expected Error: No such savepoint: receive_r0_1
      to be Error: database or disk is full
      packages/ocap-kernel/src/remotes/kernel/RemoteHandle.test.ts
      > reports the release failure rather than a missing savepoint

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No change to what any of them proves; all four still fail for the reasons
their own commits describe.

- `RemoteHandle`: assert the rollback is still *attempted*. Without this,
  deleting the rollback from the `catch` outright would turn the test green,
  which is not the fix — a `RELEASE` that failed for a reason of its own may
  well have left the savepoint standing.
- `RemoteHandle`: drop an unnecessary `as KernelStore` cast, and say why the
  store is replaced wholesale rather than having its methods assigned over
  (`makeKernelStore` hardens what it returns).
- `make-kernel`: note that `kernel-worker.ts` omits the logger too, so the wasm
  driver's pair of `logger?.error` calls stays dead even once this test passes.
  Use `vi.mocked`, as the sibling `make-kernel-options.test.ts` does.
- `causeChain` returns `Error[]`; every element is already narrowed by the loop
  guard.
- Drop "see the commit message for this test" from the four comment blocks: each
  stands alone, and the reference would not survive a squash-merge. Restate the
  claim the wasm comment made by citing a neighbouring test's title, which would
  have broken silently on rename.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`rollbackIfNeeded` was corrected for this ordering already; `commitIfNeeded`
still stepped the COMMIT first. `_inTx` is tracked in the driver rather than
read from SQLite, so a throwing COMMIT wedged it true: `beginIfNeeded` became a
permanent no-op, and the next savepoint was created bare — where its RELEASE
commits (Agoric/agoric-sdk#8423) and no later rollback could undo the delivery.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…un loop

Now that the delivery rollback spares the `crank` savepoint, `endCrank`'s
release is a real RELEASE and COMMIT on the dying path, where it used to be a
no-op. `#runLoop` called it from a bare `finally`, so a failing one silently
replaced whatever killed the kernel — and only `error.message` crosses the
wire, so the real failure reached neither `getStatus` nor the daemon log.

Report it with the crank's failure as the `cause`, the shape the rollback path
already uses. The in-flight error is boxed rather than left `undefined`, so a
crank that threw `undefined` stays distinguishable from one that did not throw.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…vepoint

`RemoteHandle.handleRemoteMessage` releases its savepoint inside the `try` and
rolls back in the `catch`. Now that a failed RELEASE discards the whole
savepoint stack, that rollback names a savepoint that is already gone and threw
`No such savepoint` out of the `catch` in place of the database failure that
brought it there — not even as `cause`.

Log the rollback failure instead of throwing it. The rollback is still
attempted, because a release that failed for a reason of its own may well have
left the savepoint standing.

`RemoteManager`'s `peerIncarnation_*` savepoint has the identical shape and had
no coverage of it at all, so a fix applied here and forgotten there would have
left its suite green. Fixed alike, and the savepoint-stack model both tests
drive the drivers with is now shared.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…llback

`rollbackCrank` gained two pieces of work that compose the wrong way round if
the failure path simply rethrows: forgetting every savepoint, and reverting the
caches a database rollback cannot reach. A failed rollback discards the whole
transaction, so the database has moved back at least as far as a successful
rollback would have taken it and those caches are at least as stale — the one
case where skipping the revert leaves the consumed GC action lost and krefs
queued for a collection that then kills the run loop.

The second test pins the other direction: reverting must not become a way to
lose the database error either.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No production call site passed one, so every `logger?.` call in the SQLite
drivers was dead code — including the aborts they report while discarding a
transaction, which fire on exactly the path where the kernel is already dying
and a diagnostic is worth most.

The browser worker has a module-level `Logger` already, so both drivers are
covered rather than only the nodejs one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The crank-transaction and rollback work landed here rather than in #1012,
which is closed and replaced. #1021 is a placeholder until the PR exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the c-list accounting fix, addressing defects found in review.

An owner that stops naming its own export left the object behind. Both the
delivered `retireExport` and the `retireExports`/`abandonExports` syscalls tore
down the owner's c-list entry but left `owner` and `refCount` in place, with no
path that could ever reclaim them: `cleanupTerminatedVat` finds krefs by walking
the owner's c-list, and the collector only revisits krefs in `maybeFreeKrefs`.
The records leaked, and the next collection to visit such a kref read the
owner's deleted entry through `getRequired` and took the run loop down with it.
New `orphanKernelObject` drops the owner mapping and hands the object to the
collector, which already knows how to retire an orphan. `collectGarbage` also
treats an owner with no c-list entry as orphaned rather than trusting the
mapping.

Reporting a dead run loop belongs to #1005, which landed on main first. It is
what makes the audit usable at all: `assertRefCountsIfAuditing` throws from
inside a crank, so with the failure logged and swallowed a violation's sole
symptom was a test hanging to its timeout with no mention of reference counts.
The `kernel-test` case here asserts that shape — the caller is told the run loop
died, and the audit error rides along as the `cause`.

Also: GC action delivery survives a vanished endpoint or a failed delivery
instead of stopping the loop; `launchVat` tears down a worker whose kernel-side
registration failed rather than stranding it; `RefCountViolation` discriminates
on `kind` instead of sentinel-matching `stored`; and the store context's
auditing flag no longer shares a name with `auditRefCounts()`.

Tests cover the crash path, the orphan-and-collect sequence, retiring
stragglers, GC-action robustness, and that a violation reaches a caller. The
`item.target` charge and both `deliver|notify` early returns now have assertions
that fail if the fix is reverted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review of the previous commit found that four of the five error handlers it
added turned a crash into a state the kernel can no longer detect. Corrects
that, and closes a hole the orphaning opened.

`orphanKernelObject` took an object's owner mapping on trust. Nothing upstream
of `performExportCleanup` checks that the vref it was handed is even an export —
`translateSyscallVtoK` maps both directions alike — so a vat could pass an
import to `abandonExports`, which needs no precondition at all, and erase a
different live vat's claim to an object it was still exporting. Sends to that
object then went splat with OBJECT_DELETED, terminating the victim tripped
`cleanupTerminatedVat`'s ownership assertion and took the run loop with it, and
the audit could not see any of it, because an export entry carries no count.
Disowning is now the owner's own doing: the expected owner is a required
argument and must match, and the syscall path rejects a mismatch outright.

The vanished-endpoint catch returned before the teardown, but
`processGCActionSet` had already consumed the action, so neither the kernel nor
the durable set remembered the object — a permanent leak, also invisible to the
audit. The kernel's side is now released whether or not anyone is left to tell,
and krefs whose entries a cleanup already removed are skipped rather than
assumed present.

The delivery-failure catch committed the teardown after the endpoint had failed
to hear about it, so the endpoint would go on to mint a fresh kref for an object
the kernel believed it had let go of — the same object with two identities. It
now aborts, which restores both the entries and the action, and terminates the
vat that could not accept the delivery.

`launchVat`'s cleanup path stopped the worker without marking the vat
terminated, so nothing ever reclaimed the records a partial launch had written.

The audit counted an importer's c-list entry as a holder during the window
between `retireKernelObjects` deleting an object and delivering the matching
`retireImport`, so the collector's own output failed the end-of-crank check. The
missing assertion in the test covering that sequence is now present.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…very

Aborting a failed GC delivery restores the action to the durable set, and
`processGCActionSet` is consulted ahead of all other run-queue work. For a vat
that is fine, because terminating it is what stops the restored action from
coming back. A remote cannot be terminated, so the same item would be selected
every crank and nothing else would ever run. A remote is a separate kernel
across a link that can drop messages anyway, and it reconciles on the next
incarnation change, so its failures no longer abort.

Also stop `orphanKernelObject` throwing on an object that is already orphaned.
Disowning something nobody owns is a no-op, not an error: only a mismatch with a
different, live owner is, which is the case the check exists for. Same for the
syscall path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sirtimid and others added 16 commits August 17, 2026 11:58
…eanup path

Both are new here and neither was reached by a test. The ownership guard is
the one that matters: nothing upstream of `performExportCleanup` checks that a
vref is even an export, so without it a vat can disown another live vat's
object. Removing the guard now fails two cases rather than none.

`launchVat`'s registration failure is covered through the store calls it
makes, since `VatManager` is hardened and cannot be spied on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Also narrows the audit's Added entry: this branch turns `RefCountViolation`
into a discriminated union, and the audit compares counts against the holders
it finds, so a holder that should have been torn down but wasn't justifies its
own count. "A leak" overstated what it can detect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t as gone

`restartVat` keeps the vat's c-list and takes the vat out of the kernel's vat
table for as long as launching a worker and negotiating with it takes. Absence
from that table was the only signal available, so a crank landing in the window
resolved a live vat as a dead one: a message went splat, a `notify` or
`bringOutYourDead` took the run loop down, and a garbage-collection action
released the kernel's side of entries the returning incarnation still holds.

The vat's flux is now recorded rather than guarded against. `provideVat` waits on
that record, so a crank arriving mid-restart delivers to the new incarnation, and
the kernel's endpoint lookup is asynchronous to let it wait. The crank waits for
the vat, rather than the restart waiting for the run loop — which is the same
direction SwingSet takes it, where a delivery to an evicted vat awaits
`ensureVatOnline` and eviction is routine. Inverted the other way, as a lock the
restart holds while the loop stands still, whatever holds it must never await
anything the loop has to deliver, and `runVat` is exactly that kind of await.

The wait for the crank in flight stays ahead of the record, which is load-bearing:
record first and wait after, and a crank that is already running reaches its
endpoint lookup, finds the record, and waits for a restart that is waiting for
that crank to end. What the ordering leaves open is a crank the run loop starts in
the turn between the wait resolving and the record appearing — it takes the
outgoing handle and can still be mid-delivery when the worker goes down. Closing
that needs the restart to happen inside a crank, the way `processUpgradeVat` does
upstream, where the vat is idle by construction and nothing mutates kernel state
from outside the run loop.

A relaunch that fails now marks the vat terminated. It previously left a vat with
no worker that the store still counted among the living, which nothing revisits:
`cleanupTerminatedVat` only walks vats that are marked.

The GC action guard for a vat that is absent but not terminated stays, now as an
assertion rather than a live path, with its reasoning corrected: aborting the
crank does preserve the action, since `rollbackCrank` restores the cached GC set,
but nothing about the vat changes between cranks, so the action would be
re-selected and re-aborted forever with no delivery to wait on.

Also shortens this PR's CHANGELOG entries, which had grown to carry rationale
that belongs in these messages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…for endpoints that are gone

Restarting a vat alongside a running run loop cannot be made safe by ordering
alone. The previous approach recorded the vat as mid-flux so a delivery would wait
for the new incarnation, and the record had to be installed *after* waiting out
the crank in flight — install it before, and a crank that is already running
reaches its endpoint lookup, finds the record, and waits for a restart that is
waiting for that crank to end. That ordering left a turn of its own: a crank the
run loop starts between the wait resolving and the record appearing takes the
outgoing handle, and can be mid-delivery when the worker goes down.

So the restart is now the run loop's own work, as a queued `restartVat` item, the
way SwingSet queues `upgrade-vat` for `processUpgradeVat`. In a crank of its own
there is no window to close: the run loop is the only thing that delivers, and it
is here instead, so the vat is idle by construction. `Kernel.restartVat` settles
when the crank has done it, and refuses outright if the run loop is dead, since
nothing would ever carry the request out.

Termination keeps the flux record, because it cannot be queued: `reset` and
`clearStorage` tear vats down on kernels whose run loop has died. Both of its
steps now live inside `#trackFlux`, in the order that does not deadlock, so a
caller does not sequence them and cannot get them wrong — with a test that hangs
if the order is reversed.

Two more, found in review of the previous round:

`#deliverNotify` and `#deliverBringOutYourDead` awaited the endpoint with no
handling for one that has vanished, so a crank landing during a termination took
the rejection into the run loop and killed it. This predates the wait — the lookup
used to throw synchronously in the same case — but the wait is what makes it
routine. All three of notify, reap, and GC-action delivery now go through
`#resolveEndpoint`, which drops the work for an endpoint that is gone for good
(a terminated vat, or a remote) and propagates anything else. The notify resolves
its endpoint before translating, which would otherwise mint c-list entries for an
endpoint with no way to hear about them.

A relaunch that failed marked the vat terminated but left its root pinned:
`stopVat` releases that pin only when it is the one ending the vat, and it had
been told the vat was coming back, while vat cleanup does not touch pins at all.
The pin, and the root's refcount, were held for the life of the kernel. Both
paths now release it through one helper.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The send path caught every endpoint lookup failure and treated it as a splat,
which its own TODO called out: an error that is not "this endpoint is gone"
silently discarded a deliverable message and rejected its result with
ENDPOINT_UNREACHABLE. It is now the last of the four delivery paths to go through
`resolveEndpoint`, so a splat happens where the endpoint will not be back — a
terminated vat, or a remote — and anything else propagates.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd rollback

Four ways to kill or wedge the kernel, found reviewing this branch.

`rollbackCrank` emptied `maybeFreeKrefs` rather than restoring it. The set is
not per-crank — only `collectGarbage` empties it, at the end of a crank that had
an item — so a candidate created while the run loop was idle, as `terminateVat`
unpinning a root creates one, was owed a collection that any later crank's
rollback silently cancelled. Savepoints now carry the set as it stood when they
were taken. The audit cannot see this one: the counts stay self-consistent at 0.

A restart that could not relaunch its vat threw, and the run loop's catch rolls
back on any throw — undoing the termination records `performVatRestart` had just
written and returning the request to the run queue. Every subsequent process
start dequeued it and failed the same way. It now terminates the vat and reports
through the waiter, so the crank commits and the request is spent. The comment
claiming the throw preserved those records had the causality backwards.

Terminating a vat left a queued restart for it to be carried out against a vat
that no longer existed; `#restartVatWorker` is the one item type that does not
go through `#resolveEndpoint`, so the resulting `VatNotFoundError` propagated.
Restart-then-terminate is reachable from RPC. The waiter is now rejected when
the vat is terminated and the request dropped when the crank reaches it.

`cleanupTerminatedVat` ends by *unmarking* the vat it finished, so work
outliving it — a `bringOutYourDead` scheduled before it died, which nothing
purges from the reap queue — arrived at an endpoint that was neither present nor
terminated, which `#resolveEndpoint` reserves its throw for. It now asks whether
the store has a live record of the vat at all.

Also fixed, from the same review:

- `getImporters` counted only vats, so retiring an object deleted it without
  telling a remote importer, leaving a c-list entry naming nothing — which the
  audit reports as dangling, taking the run loop with it. Adds `getRemoteIds`.
- `#deliverGCAction` computed the live kref set before awaiting the endpoint and
  used it after. A remote re-handshaking in that window clears its c-list
  without waiting for the crank, and `krefsToErefs` throws rather than returning
  short.
- `#endVat` marks the vat terminated in a `finally`. A teardown that threw left
  it unmarked, which is the state above, and falsified `#trackFlux`'s stated
  invariant that waiters can read "gone" as terminated.
- Comments that no longer described the code: `provideVat` waiting on restarts
  (only teardown is recorded), `stopVat` tearing down "only the worker" (it
  releases the root pin, as of this branch), `clearStorage` terminating vats,
  the audit standing in for the disabled `retireExport` assert, and a stale
  `(1, 1)` baseline rationale. `#vatsInFlux` narrows to `Promise<void>`, which
  removes a branch of `provideVat` that could not be reached.

Tests: each fix has a regression test that fails against the code without it.
Closes the two coverage gaps the review named — the splat path charging the run
queue item's own target when routing went through a promise, and `ko6.refCount`
in the control-panel e2e, restored as three per-checkpoint values rather than
dropped as nondeterministic. Full unit suite, kernel-test with auditing on every
crank, and `test:e2e:ci` at 17/17.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only `deleteVat` removes `vatConfig.<vatId>`, and `cleanupTerminatedVat`
sweeps `${vatId}.`-prefixed keys, which never match it. The writes making up
a vat's death were interleaved with awaits across `VatManager.stopVat`,
`#endVat`'s `finally`, `VatHandle.terminate` and a lambda in `Kernel.ts`, so a
throw part-way left the vat marked terminated with its config alive — which
reads as *active* again as soon as cleanup drops the mark, killing the run
loop over the disagreement and resurrecting the vat on the next process start.

`VatManager.#retireVat` now makes all four writes with no await between them,
modelled on SwingSet's synchronous prelude in `kernel.js` `terminateVat`;
worker teardown follows and is best-effort. `#endVat` and `#abandonVat` go as
duplicates of it, and `VatHandle.terminate` is left with only its own channel
to close.

A vat whose stream fails is retired by the manager, via a new
`onCriticalFailure`, rather than tearing itself down: that left the handle in
the manager and the vat live in the store, so the next delivery went to a
worker that could not answer and, the vat RPC client having no timeout, the
crank never completed while the run loop still reported itself running.

`makeGCAndFinalize` drains the queues before sweeping, since a pending
continuation still holds its closure's objects, so a vat reports its dropped
imports on the `bringOutYourDead` that provoked them rather than a later one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tial launch terminated

Two conflicts the rebase onto the GC-hardening stack surfaced, both real
disagreements rather than textual ones.

`revertStateBeneathRollback` cleared `maybeFreeKrefs` outright. The set is not
per-crank — only `collectGarbage` empties it — so a candidate added while the
run loop was idle, which `terminateVat` unpinning a root produces, was owed a
collection and lost it to an unrelated crank's rollback. It now restores the
savepoint's snapshot, which discards the abandoned crank's additions and keeps
everything that predates it. The unit test had encoded the old behaviour and is
updated to distinguish the two cases.

`launchVat`'s cleanup relied on `stopVat` reaching `#retireVat` to record the
death, but `stopVat` refuses a vat the kernel has no handle for and the store
does not call active — which is what a partial launch looks like. The mark is
asserted directly again, as it was before this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Recording the vat's death only saves the deliveries that come after it. The one
in flight when the worker died stays parked on an RPC client with no timeout, so
its crank never completes — the same hang `onCriticalFailure` exists to prevent,
one delivery earlier. The worker was left running too, since nothing else would
stop it once the handle was off the books.

Found by Cursor Bugbot on #1023.

Also reverts this branch's additions to the extension control-panel e2e test.
They asserted `ko6.refCount` directly, and which vat owns `ko6` is not stable:
order. The behaviour they checked is covered by the refcount audit, which runs
on every kernel `kernel-test` builds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… exists

`onCriticalFailure` closed over the binding `VatHandle.make` was still to return, so a stream that broke during `#init` threw on the temporal dead zone — after the vat had been retired, and before anything rejected the pending `initVat` that nothing else will ever settle. The handle is now passed to the callback, and `runVat` refuses to put a handle on the books for a vat retired while it was being made.

The teardown also awaited the worker kill before `terminate`, which is what rejects the vat's pending RPCs. A worker slow to die — or one that never does — kept the parked delivery parked, which is the hang this path exists to clear. The two now run alongside each other, `terminate` first.

Found by Cursor Bugbot on #1023.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…point

`KernelQueue.#runLoop` calls releasing its `crank` savepoint "this crank's one
commit point". `releaseAllSavepoints` releases `t0`, which is the outermost
savepoint only when the crank opened the first one, and two production paths
open savepoints through `KernelStore.createSavepoint` -- invisible to the
ordinal numbering, uncoordinated with the crank, one of them held across an
await.

Real SQLite through the real driver, one test per interleaving.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A failed `ROLLBACK TO` is taken to discard the whole transaction, which is what
makes truncating the savepoint list to zero match the database. That holds only
when the compensating abort succeeds, and both drivers catch and log one that
does not. This driver reads `db.inTransaction` from SQLite, so it cannot wedge a
flag -- and also cannot end an ownerless transaction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Clearing `_inTx` before stepping the COMMIT stops a throwing COMMIT wedging the
flag true, and leaves nothing able to end the transaction it left open:
`rollbackIfNeeded` reads the false flag and returns, and `releaseSavepoint`
reaches `commitIfNeeded` with nothing wrapping it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s callers

The flush is last of the crank's own work so that no external caller is answered
before the fallible work is done, and the reference count audit runs after the
flush so that buffered items are not read as leaks. The audit is itself
fallible, so the two orderings contradict each other.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`maybeFreeKrefs` is not per-crank: only `collectGarbage` empties it, so a
candidate added while no crank was open must survive an unrelated crank's
rollback. `RemoteManager.#handlePeerIncarnation` is such a producer, and the
objects it abandons are invisible to the reference count audit once lost.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@grypez

grypez commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Claim 1 — "releasing crank in endCrank is this crank's one commit point"

What the claim is

KernelQueue.#runLoop opens two savepoints per crank:

this.#kernelStore.createCrankSavepoint('crank');
this.#kernelStore.createCrankSavepoint('delivery');

with the reasoning that rolling back the outermost savepoint discards the
enclosing transaction, and an aborted crank still has writes to make
(#terminateVat, collectGarbage). So delivery absorbs the rollback and
crank survives it. endCrank then releases crank, and that release is the
crank's single commit point
: exactly one place where a crank's work becomes
durable, exactly one place where it can be lost wholesale.

Why it matters

The claim is the whole thesis of #1021. If it holds, a crank is atomic: either
the delivery, the termination bookkeeping, and the flush all land, or none of
them do, and a restart resumes from a coherent point. If it does not hold, then
"one transaction per crank" is a description of the intended shape rather than
an invariant, and the failure mode is the one this PR series exists to remove —
a crank half in the database.

How to evaluate it

The claim is about the database's savepoint stack, not ctx.savepoints. Two
questions:

  1. Is t0 always the outermost savepoint on the connection? releaseAllSavepoints
    hardcodes kdb.releaseSavepoint('t0'), and createCrankSavepoint names by
    ordinal (t${ctx.savepoints.length}), so t0 is outermost only if the crank
    opened the first savepoint on the connection.
  2. Can anything else open a savepoint the crank cannot see?

For (2), KernelStore exposes createSavepoint/releaseSavepoint/
rollbackSavepoint (store/index.ts:305-325) which call kdb directly and
never touch ctx.savepoints. Two production callers:

  • RemoteHandle.handleRemoteMessagereceive_${remoteId}_${seq}, opened at
    line 1008 and held across await this.#handleRedeemURLRequest(...)
  • RemoteManager.#handlePeerIncarnationpeerIncarnation_${peerId}, line 242

Neither calls waitForCrank or checks isInCrank. Kernel.#init installs the
remote message handler as a bare async callback with no crank coordination, and
the run loop spends its time in await deliver(queueItem). So the two stacks
interleave, and all three relative orderings are reachable.

Then ask what SQLite actually does in each ordering. That is a question about
savepoint semantics, so it should be answered by SQLite and not by a mock — note
that the driver's own tests stub db.exec entirely, so nothing in the existing
suite exercises real SAVEPOINT/RELEASE/ROLLBACK TO behaviour.

nodejs.savepoint-interleaving.test.ts runs the real driver against an in-memory
better-sqlite3, one test per ordering.

Outcome: claim is false. Three failures.

Ordering What happens
Remote savepoint outside the crank RELEASE t0 leaves _spStack = ['receive_r1_7'], so commitIfNeeded does not fire. No commit. The remote's later ROLLBACK TO then discards the entire "committed" crank. Silent.
Remote savepoint inside, delivery aborts ROLLBACK TO t1 cancels every savepoint started after t1, including the remote's. The remote handler gets No such savepoint: receive_r1_7. Loud and recoverable — the best of the three.
Remote savepoint inside, crank succeeds RELEASE t0 releases everything above it, committing the remote's half-finished message — it is parked on its await and has not reached setRemoteHighestReceivedSeq. The handler then fails, so the peer retries an effect that has already landed. Exactly-once is broken.

The first row is the serious one: it is the exact failure this PR series set out
to eliminate, reachable without any I/O error, and it is silent.

This is pre-existing, not a regression — the raw-savepoint bypass predates
#1021, and these tests fail against main, #1021, and this tip alike. But the
comment in #runLoop states an invariant the code does not have, and that
comment is what the next person will build on.

What would make the claim true

Any one of: route the two remote paths through createCrankSavepoint so they
join the ordinal numbering; make them await waitForCrank() and assert
!isInCrank() before taking a savepoint; or have releaseAllSavepoints release
ctx.savepoints[0] by identity and assert it is the bottom of kdb's stack, so
a foreign savepoint underneath fails loudly instead of quietly deferring the
commit. Failing all of those, the comment should say that crank's release is
the crank's intended commit point and name what can sit underneath it.

@grypez

grypez commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Claim 2 — flush after the fallible work, audit after the flush

What the claim is

Two ordering decisions in #processCrankResult, each with its own stated
reason:

this.#kernelStore.collectGarbage();
if (!crankResult?.abort) {
  // After the fallible work above, not before it. The flush settles the
  // promise `enqueueMessage` gave an external caller, so a later rollback
  // would discard the state that answer was computed from.
  this.#flushCrankBuffer();
}
// After the flush, because the audit reads the run queue as ground truth
// while a buffered item's references were already counted when it was
// enqueued: audited mid-flush, every buffered item reads as a leak.
this.#kernelStore.assertRefCountsIfAuditing();

So: flush last, because answering an external caller is irreversible and must
not precede anything that can still roll back. And audit after the flush,
because the audit's ground truth is the persisted store, which does not include
the in-RAM crank buffer.

Why it matters

The first ordering is a correctness property with no test coverage and no type to
protect it — it lives entirely in the sequence of statements. The second is what
makes auditRefCounts usable at all; get it wrong and the option is a false-alarm
generator that kills the run loop on healthy state.

How to evaluate it

Take the audit premise first. Read what auditRefCounts treats as ground truth:
computeExpectedRefCounts walks getPrefixedKeys('') and credits only
${endid}.c.${eref} c-list rows, queue.* entries, ${kpid}.state, resolution
slots, and pins (refcount-audit.ts:182-261). All KV. ctx.crankBuffer is a
plain array in RAM and appears nowhere. Meanwhile enqueueSend/enqueueNotify
increment refcounts before choosing buffered or immediate. So pre-flush, every
buffered item has paid for references that no visible holder accounts for →
"stored too high" → violation. The premise is correct.

Then the flush ordering. It is not enough that the flush is the last statement
that answers callers
; the question is whether anything after it can still undo
the crank, and whether anything before it already answered. Two probes:

  1. Does anything before the flush settle an external subscription? Yes, and this
    branch documents it: #terminateVat resolves the dying vat's promises via
    resolvePromises (VatManager.ts:334), whose immediate defaults to true,
    invoking their subscriptions before collectGarbage. The comment says so
    outright — "Not airtight". #processCrankResult also rejects the aborted
    send's result subscription directly, earlier still.
  2. Does anything after the flush roll the crank back? This is the untested one.
    assertRefCountsIfAuditing throws. On the success path #crankRollbackAttempted
    is false, so the run loop's catch runs rollbackCrank('delivery').

KernelQueue.audit-ordering.test.ts drives one crank with a buffered notify, a
kernel subscription on its kpid, and an audit that reports drift, then asserts
what happened in which order.

Outcome: premise true, ordering rationale defeated — one failure.

The audit premise is confirmed by reading; no test needed and none written.

The flush-last rationale does not survive. The test shows the subscription's
resolve is invoked by the flush, and then rollbackCrank('delivery') is called —
the caller has its answer and the state that answer was computed from is
discarded underneath it. That is verbatim the hazard the comment above the flush
says the ordering prevents.

The two orderings are in direct conflict:

  • put the audit before the flush and it reports every buffered item as a leak;
  • put it after and it becomes fallible work running after the answers have gone
    out.

This branch picked audit-last and thereby gave up the property the flush-last
comment claims. Nothing pins it: KernelQueue.test.ts mocks
assertRefCountsIfAuditing as a no-op, so no existing test ever takes the throw.

Worth noting this is new. On main the audit throwing led to
rollbackCrank('start'), which discarded the whole transaction — the answers were
wrong there too, but the shape was different. The two-savepoint change altered
what a post-flush throw does without the ordering being revisited.

What would make the claim true

The audit does not need to see a consistent store — it needs to not be fooled by
the buffer. Teach computeExpectedRefCounts to credit ctx.crankBuffer items
the same way it credits queue.run.* rows, and the audit can run before the
flush, where a throw still rolls back cleanly and no caller has been answered.
That resolves the conflict rather than picking a side. Short of that, the
flush-last comment should be narrowed to what it actually buys, the way the
#terminateVat gap already was.

@grypez

grypez commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Claim 3 — a failed ROLLBACK TO discards the whole transaction, in both drivers

What the claim is

rollbackCrank, on a rollback that throws:

} catch (error) {
  // A failed rollback discards the whole transaction, so every savepoint
  // is gone, not just this one. Truncating to `ordinal` would have
  // `endCrank` release a `t0` the database lacks and throw over whatever
  // really killed the kernel.
  ctx.savepoints.length = 0;

The claim: a failed ROLLBACK TO leaves no transaction and therefore no
savepoints, so emptying ctx.savepoints keeps the in-memory list in step with
the database — and this is true of nodejs.ts and wasm.ts alike.

Why it matters

ctx.savepoints is not bookkeeping. Its length is the ordinal the next
savepoint is named by, and releaseAllSavepoints releases t0 iff it is
non-empty. If the list says zero while the database still has savepoints and an
open transaction, then the next crank creates a second t0 inside the surviving
transaction, and the invariant "empty stack ⇒ nothing to commit" — which is
exactly what commitIfNeeded keys on — is false. Every later write on the
connection joins a transaction nobody owns.

How to evaluate it

The claim is about the drivers, so read what each does when ROLLBACK TO throws.
Both are the same shape:

db._spStack.length = 0;
try {
  rollbackIfNeeded();
} catch (abortError) {
  logger?.error('failed to discard transaction after rollback', abortError);
}
throw error;

The transaction is discarded by rollbackIfNeeded, i.e. by a ROLLBACK TRANSACTION that is itself caught and logged if it fails. So the claim's
"⇒" is conditional on that abort succeeding — and the code, by catching, states
that it might not. (#1021 added exactly this logging, so the possibility is
acknowledged, not hypothetical.)

That gives a precise question: what state are we in when both the rollback and
the abort fail, and does anything later depend on it? Follow the consequence
rather than stopping at the state:

  • nodejs: db.inTransaction is read from SQLite, so it stays true.
    _spStack is []. beginIfNeeded sees the live transaction and skips
    BEGIN, so the next savepoint is created inside it; when that savepoint is
    released the stack empties and commitIfNeeded fires.
  • wasm: rollbackIfNeeded sets _inTx = false before stepping the abort,
    so a throwing abort leaves the flag false with a live transaction. The next
    beginIfNeeded issues a BEGIN that SQLite rejects.

Is there a "next savepoint" after the run loop has died? Yes — assertRunLoopAlive's
own doc comment says teardown must not be refused, and reset, terminateAllVats,
a peer incarnation change and a remote message all take savepoints.

nodejs.transaction-survival.test.ts drives the double failure and then does what
teardown does.

Outcome: claim is conditionally false — two failures.

The invariant holds whenever the compensating abort succeeds, which is the
overwhelmingly common case. When it does not:

  • nodejs issues COMMIT TRANSACTION on the transaction that was supposed to
    have been discarded, at the next teardown savepoint's release. The abandoned
    crank is committed. Silent.
  • wasm fails loudly at the next BEGIN.

Two drivers, opposite desynchronisations, only one of them noisy. So the second
half of the claim — "true for both drivers" — is where it breaks: they do not
degrade the same way, and the crank layer above them is written as if they do.

Severity, honestly

This needs a double I/O failure to reach, and after it the kernel is dead either
way. I would not block on it. But two things follow that are worth writing down:
the comment should say "discards the transaction if the abort succeeds", and
the divergence between drivers under the same fault is the same asymmetry claim 6
turns on, from the other end.

Interaction with claim 5

See that comment: reverting the caches on this path is safe in isolation, but
combined with the state above the caches say "rolled back" while the database
still holds the abandoned writes and may yet commit them.

@grypez

grypez commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Claim 4 — every kref in maybeFreeKrefs is there because this crank decremented it

What the claim is

As stated on #1021, where revertStateBeneathRollback ends with:

// Clearing all of them is correct only while a rollback discards the whole
// delivery, which is all any caller asks for.
ctx.maybeFreeKrefs.clear();

The justification for clear() is that the set is effectively per-crank: its
entries are collection candidates only because of decrements this crank made,
and the rollback just undid those decrements, so the entries are meaningless and
dropping them is not a loss.

Why it matters

maybeFreeKrefs is the only record that a kref might be collectable. Nothing
recomputes it — collectGarbage consumes and empties it, and krefs enter only by
being added at the moment of a decrement, a clearReachableFlag, or a c-list
teardown. A kref dropped from the set is not rediscovered later. It becomes an
object with no owner, no c-list entry, a zero refcount and no path to deletion.

And that leak is invisible to the auditor: auditRefCounts compares stored
counts against credited holders, and an orphan with zero holders and a stored
count of 0,0 matches. So this is the rare failure with no downstream signal at
all.

How to evaluate it

The claim is a universally-quantified statement about set membership, so it
falls to a single counterexample: a kref in maybeFreeKrefs that got there
outside this crank. Enumerate the producers (git grep 'maybeFreeKrefs.add'):

  • refcount.ts:164,180decrementRefCount
  • reachable.ts:105clearReachableFlag
  • vat.ts:267cleanupTerminatedVat
  • vat.ts:396forgetEndpointImports

and then ask, for each, whether it can run with no crank open. forgetEndpointImports
answers immediately: its only caller is RemoteHandle.persistPeerRestart, called
from RemoteManager.#handlePeerIncarnation, which runs from a network callback,
takes its own peerIncarnation_${peerId} savepoint, and calls no
collectGarbage. So the krefs it adds are committed-and-orphaned, sitting in the
set, waiting for whichever crank next harvests them.

Then: if that crank rolls back, are they still there?

crank.cross-crank-gc.test.ts orphans a remote export exactly as a peer restart
does, and runs cranks around it — one that succeeds (control), one that rolls
back, and a rollback followed by five clean cranks.

Outcome: false against #1021, true here. Three passes.

Against #1021's head the control passes and the other two fail: the orphan
survives the rollback, and no number of later cranks ever collects it. Permanent,
silent leak.

Against this branch all three pass. eaa71ac00 replaced the clear with a
snapshot-and-restore:

ctx.savepoints.push({ name, maybeFreeKrefs: new Set(ctx.maybeFreeKrefs) });
...
ctx.maybeFreeKrefs.clear();
for (const kref of restored.maybeFreeKrefs) {
  ctx.maybeFreeKrefs.add(kref);
}

and its comment states the corrected rule directly — "the set is not per-crank:
only collectGarbage empties it, so a candidate added while the run loop was
idle — terminateVat unpinning a root is the real path — is still owed a
collection and must survive an unrelated crank's rollback."

That is the right fix and it is already made. These three tests are offered as a
regression guard on it: they pass here and fail against #1021's head.

One thing to carry across

#1021 still has the clear(). Since these branches do not descend from one
another, whichever merges second needs the snapshot version, not the clear —
this is the mirror image of the wasm commitIfNeeded situation in claim 6, where
the fix exists on #1021 and is missing here.

Also worth pinning if you want belt and braces: #1021's own
crank.test.ts test 'reverts the caches the database cannot reach even when the rollback fails' seeds kp1 into maybeFreeKrefs before creating the
savepoints and asserts the set ends up empty — i.e. it asserts the pre-crank kref
is discarded. Under the new semantics that assertion is wrong, and it should be
updated to expect kp1 to survive rather than deleted.

@grypez

grypez commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Claim 5 — reverting the caches is safe on the failed-rollback path too

What the claim is

rollbackCrank calls revertStateBeneathRollback on both exits, and the failed
one is deliberate:

} catch (error) {
  ctx.savepoints.length = 0;
  // Before the rethrow, and not only on the path below. A failed
  // rollback discards the whole transaction, so the database has moved
  // back at least as far as a successful rollback would have taken it
  // and these caches are at least as stale.
  revertStateBeneathRollback(restored, error);
  throw error;
}

The argument is monotonic: a failed rollback discards the whole transaction,
which is further back than the savepoint asked for, so caches built over the
abandoned crank are at least as stale as on the success path, and refreshing them
can only help.

Why it matters

This is the one place in the series that deliberately does work on the way out of
a fatal error. If it were wrong it would be wrong precisely when the operator has
least information — a database failure, a dying run loop, and now caches mutated
on the way past. The alternative (rethrow immediately) is the obvious-looking
choice, and the comment exists to say why it is worse: it would leave the dying
crank holding the GC action it consumed and the krefs it was about to collect.

How to evaluate it

The claim is a comparison, so enumerate the reachable database states after a
failed ROLLBACK TO and ask, for each, whether refreshing is worse than not:

  1. Rollback failed, abort succeeded (the normal case). The transaction is
    gone; the database is back past t0. Every cache re-read comes from committed
    state. Refreshing is not merely safe, it is mandatory — provideCachedStoredValue
    answers from a closure and writes through, so an unrefreshed closure would
    persist the abandoned value on its next set.
  2. Rollback failed, abort also failed (claim 3's hole). The database did
    not move back; the abandoned writes are still visible on this connection.
    refreshCachedValues re-reads them — the same values the caches already held.
    No change, so no harm.
  3. Partially rolled back. Not a state SQLite can produce; ROLLBACK TO is
    atomic.

So there is no state in which the refresh makes the caches worse. The restored
snapshot in state 2 is the interesting sub-case, but it restores
maybeFreeKrefs to a point in the past, which is conservative in the right
direction — see claim 4.

Outcome: claim holds. No test written; nothing to pin.

I could not construct a counterexample, and I do not think one exists at the
level the claim is pitched. The reasoning is sound and the comment states it
accurately.

The caveat worth recording

Claim 5 is safe about the caches. It is not a statement about the system, and
in state 2 above the combination is genuinely bad:

  • the caches have been reverted, so in-memory state says "this crank did not
    happen";
  • the database still holds the crank's writes in a live transaction, and per
    claim 3 the next teardown savepoint's release will commit them on the nodejs
    driver.

Caches say rolled back, disk says committed. Neither claim 3 nor claim 5 is
individually false, and the conjunction is a split brain. That is an argument for
fixing claim 3's hole (abort failure should be terminal for the connection, not
logged and stepped over), not for changing anything here.

One smaller note: because a failed delivery rollback discards the whole
transaction, writes the two-savepoint design deliberately places in crank are
lost on that path too — the crank savepoint buys nothing when delivery's
rollback fails. In #processCrankResult the ordering saves us: rollbackCrank
precedes #terminateVat, so a failed rollback throws before the worker is
killed, and the store is not left believing a dead vat is alive. Worth a comment
at the #terminateVat call, since the current one explains why its writes must
outlive the rollback without noting that on the failed path they do not exist yet.

@grypez

grypez commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Claim 6 — commitIfNeeded clears _inTx before the COMMIT; wasm clears before the abort

What the claim is

Two related driver changes. In wasm.ts, _inTx is cleared before the
statement it guards, in both commitIfNeeded and rollbackIfNeeded, with the
reason given at rollbackIfNeeded:

// Cleared before the abort, which can throw: left true, `beginIfNeeded` is
// a no-op forever after and writes autocommit one statement at a time (see
// `createSavepoint`).
db._inTx = false;

The claim: clearing first is strictly better, because a throwing statement cannot
then wedge the flag true and turn beginIfNeeded into a permanent no-op —
after which savepoints are created bare, their RELEASE autocommits
(Agoric/agoric-sdk#8423), and no rollback can undo a delivery.

Why it matters

_inTx is the wasm driver's entire model of whether a transaction exists. Both
commitIfNeeded and rollbackIfNeeded gate on it, so if it disagrees with
SQLite, one of two things happens: the driver thinks there is a transaction when
there is none (every commit/abort throws), or it thinks there is none when there
is (nothing will ever end it, and every write joins it and reports success).
nodejs.ts has no such flag, reading db.inTransaction from SQLite, and #1021's
b90e7a5e5 cites that as the reason the two drivers differ — filed as #1013.

How to evaluate it

The falsification to look for is the other direction: a throw between the clear
and the statement, leaving _inTx wrong the opposite way.

Clearing first closes the wedge and opens the reverse hole, and that hole is
reachable without any exotic assumption: SQLite can fail a COMMIT with the
transaction still open.
Trace wasm.releaseSavepoint on this branch:

  1. db.exec('RELEASE SAVEPOINT t0') succeeds
  2. _spStack.splice(idx) empties the stack
  3. commitIfNeeded()_inTx = false, then COMMIT TRANSACTION throws

Now _inTx is false with a live transaction. rollbackIfNeeded reads the false
flag and returns. commitIfNeeded will not run again until some savepoint release
empties the stack. And note step 3 is reached from outside the try/catch that
wraps the RELEASE, so nothing catches it there either. The transaction has no
owner — the exact hazard rollbackSavepoint and releaseSavepoint already
discard the transaction to avoid, reached by a third door.

So the evaluation is: for each driver, does commitIfNeeded leave a transaction
behind when the COMMIT fails? wasm.transaction-survival.test.ts and the second
case of nodejs.transaction-survival.test.ts assert that an abort follows.

Outcome: both drivers fail. Two failures, and the fix already exists on #1021.

That second one contradicts b90e7a5e5's stated reason for treating this as
wasm-only:

The nodejs driver reads db.inTransaction from SQLite rather than caching it,
so it has no equivalent gap; that asymmetry is #1013.

Reading the flag from SQLite prevents the wedge. It does not prevent the
ownerless transaction, which is the half that loses data — and on nodejs it is
worse, because inTransaction staying honest is exactly what lets the next
teardown savepoint join the doomed transaction and COMMIT it (see claim 3).

So: the claim is true of wasm as written on #1021, false of wasm as it stands
here, and the parenthetical about nodejs is false everywhere.

What to do

  1. Carry b90e7a5e5 onto this branch, or land fix: keep a crank's store work inside one transaction #1021 first and rebase. Whichever
    merges second needs it.
  2. Give nodejs.commitIfNeeded the same treatment wasm's got, and drop the
    "no equivalent gap" note from the changelog and Extract the duplicated sqlite savepoint methods into one module #1013.
  3. Consider whether the flag is worth keeping at all. nodejs gets by without
    one, and a driver that asks SQLite cannot disagree with it.
    sqlite3_get_autocommit is exported by the wasm build
    (@sqlite.org/sqlite-wasm/sqlite-wasm/jswasm/sqlite3.mjs) though it is absent
    from the package's index.d.ts, so this would need a typing shim rather than
    just a call — worth weighing against carrying _inTx correctly forever.

@sirtimid
sirtimid force-pushed the sirtimid/vat-lifecycle-consistency-v2 branch 2 times, most recently from 5d9bd14 to f624a34 Compare September 11, 2026 13:06
@sirtimid

Copy link
Copy Markdown
Contributor

Three things from working through this against the stack, so you don't have to rediscover them.

Claim 4 moved down to #1021. crank.cross-crank-gc.test.ts was the one file of yours that never got adopted, and it was the one that mattered most: it fails 2 of 3 against #1021 as it stood, because only the tip had the snapshot-and-restore. The bare ctx.maybeFreeKrefs.clear() would have landed on main when #1021 merged and stayed there until #1023 followed. Both the fix and your test are now in #1021, and the guard is what caught it — it reproduces exactly 2/3 against a mutation back to the clear.

Two of your tests were changed rather than adopted, and are worth a second look:

  • KernelQueue.audit-ordering.test.ts was retitled from your "does not answer the external caller it is about to roll back" to "keeps the delivery it already answered the caller from". That inverts what it asserts. It now pins the accepted trade-off rather than the property you asked for, so if you still want the original property it needs raising again rather than reading the file.
  • nodejs.savepoint-interleaving.test.ts's assertions were deliberately changed: yours asserted not.toThrow() and that half-done work is durable; they now record what SQLite actually does.

Claim 2 is still open and now has an issue. computeExpectedRefCounts still doesn't credit ctx.crankBuffer, so the audit-versus-flush conflict stands and only the comment was narrowed. Filed as #1062.

This branch is based on a base that no longer exists (merge-base 180e6ac47), so its diff shows 72 files and the per-claim measurements are against something else now. It wants either a rebase onto the current sirtimid/vat-lifecycle-consistency-v2 or closing as absorbed — four of your five files are in with your cases intact, and the fifth is the cross-crank one that just went into #1021.

sirtimid added a commit that referenced this pull request Sep 14, 2026
…e the gate's own gaps

Three changes to the crank layer, all of which #1021 either introduced or
left open:

`rollbackCrank` restores `maybeFreeKrefs` to the savepoint's snapshot rather
than emptying it. That set is not per-crank — only `collectGarbage` empties
it — so a candidate produced outside a crank, as a peer restart abandoning a
remote's exports does, was dropped by an unrelated crank's rollback and the
objects leaked with nothing left to notice them. The reference count audit
cannot see it either: an orphan with no holders and a count of zero looks
consistent. Guard adopted from #1039.

`RemoteManager` snapshots the restarting peer's promises inside its turn at
the store rather than before waiting for one. The wait spans a whole crank,
so a promise that crank made the peer decider of was never rejected and the
sending vat waited on it forever. This one was #1021's own regression.

`beginOutOfCrank`/`endOutOfCrank` are replaced by `withStoreOutOfCrank`. A
turn that was never given back left the run loop waiting on a promise nothing
resolves — no failure, no log, no timeout — and the callback's type now says
the held section has to be synchronous.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sirtimid
sirtimid force-pushed the sirtimid/vat-lifecycle-consistency-v2 branch 3 times, most recently from c545f60 to 2005001 Compare September 15, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants